Skip to content

Fix NH-3252 #204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Fix NH-3252 #204

wants to merge 2 commits into from

Conversation

darrenkopp
Copy link
Contributor

This is similar to the fix proposed on jira for NH-3252. Basically size is handled the same way as string. Without this, can't do varchar(max), only nvarchar(max).

Handle ANSI strings in the same way that normal strings are handled.
@hazzik
Copy link
Member

hazzik commented May 9, 2013

@darrenkopp, can you please add unit tests?

@darrenkopp
Copy link
Contributor Author

@hazzik Yes, I will add some tests for this. Saw this a week ago and haven't had time to do it, so just dropping this comment to let you know that I am aware and will add it when I get some time.

Add tests. Had to use old hbm.xml method since length gets ignored on
new by code stuff
@darrenkopp
Copy link
Contributor Author

@hazzik ok, added tests, and found new problems with by code mapping in the process XD. Everything here works though

@oskarb oskarb added this to the 4.1.0 milestone Sep 25, 2014
@hazzik
Copy link
Member

hazzik commented Nov 18, 2014

@darrenkopp what about fixing the issue you've found with mapping by code? (or at least please create a JIRA issue)

@darrenkopp
Copy link
Contributor Author

Yeah... at the time didn't have time to do it, so never created a test covering it nor do i remember exactly what the issue was. I'll re-create a test for this using the by-code mapping method and I'll likely re-remember what the issue is and add a new jira ticket.

It was either ignoring the length I was giving it, or it was not allowing ansistring, but I'm not sure exactly which one.

@hazzik
Copy link
Member

hazzik commented Feb 22, 2015

Merged @ bea9bc9

Thanks, @darrenkopp

@hazzik hazzik closed this Feb 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants